feat(organizations): owner_add lifecycle + addMember invitee email#3858
Conversation
declineMembership mirrors acceptMembership's consent gate (pending + owner_add + caller is the invitee) and deletes the row so the user can be re-invited. remove() now runs last-owner protection only for ACTIVE owner rows — cancelling a pending owner-role invite in a 1-active-owner org no longer throws. list() surfaces pending owner_add rows next to active members so the inviting owner can see and cancel an invite; pending join_requests keep their own approval surface. refs #3831
remove() iterates listByUser, which is ACTIVE-only, so a deleted user's PENDING rows (join_request and owner_add) survived as orphans pointing at a dead userId. Sweep them explicitly after the active-membership processing. refs #3831
DELETE /api/membership-requests/:membershipId — auth-only chain like the accept route; the consent gate lives in the service and answers 404 on any mismatch without leaking which condition failed. Registered after /mine and /mine/pending so those literals are never captured as an id. Makes the 'Please accept or decline it' join-request copy honest, proven end-to-end by the new e2e flow. refs #3831
addMember created a pending owner_add membership silently while all three join-request paths email their counterpart — an org-less invitee got zero signal. Notify the invited user via the new org-member-added template with invitation wording (the membership stays PENDING until THEY accept, consent invariant #1), fire-and-forget with logger.warn on failure, guarded by mailer.isConfigured() like the existing membership emails. refs #3832
Gate the OrganizationRepository.get behind mailer.isConfigured (matches approveRequest/rejectRequest) — no needless DB read on the mail-off path. refs #3832
|
Warning Review limit reached
More reviews will be available in 47 minutes and 2 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
WalkthroughThis PR implements the owner_add invitation decline feature. It adds an email template for invitations, updates addMember to send emails, introduces a declineMembership service function with consent gating, wires HTTP endpoints for the decline action, updates the members list filter to show pending invitations, narrows the last-owner protection to ACTIVE members only, sweeps pending memberships on user deletion, and includes comprehensive unit and E2E tests. ChangesOrganization owner_add invitation decline flow
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #3858 +/- ##
==========================================
+ Coverage 92.03% 92.37% +0.33%
==========================================
Files 160 160
Lines 5337 5361 +24
Branches 1717 1723 +6
==========================================
+ Hits 4912 4952 +40
+ Misses 337 328 -9
+ Partials 88 81 -7
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR completes the Node-side of the owner_add membership lifecycle by adding an invitee-decline endpoint, fixing last-owner protection scope, and exposing pending owner_add rows on the members list. It also adds a fire-and-forget invitation email when addMember() creates a pending invite, and ensures user deletion cleans up orphanable pending membership rows.
Changes:
- Add
DELETE /api/membership-requests/:membershipId+MembershipService.declineMembership()with the same consent gate semantics as accept (opaque 404 on mismatch). - Adjust membership listing/removal behavior: list includes pending
owner_add, and last-owner guard applies only toACTIVEowners. - Add invitee notification email on
addMember()(mailer-configured only) and sweep pending memberships during user removal, with comprehensive unit/e2e test coverage.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| modules/users/services/users.service.js | Sweeps deleted user’s pending memberships to prevent orphaned pending rows. |
| modules/users/tests/users.service.remove.pendingSweep.unit.tests.js | Unit tests validating pending-membership sweep on user deletion. |
| modules/organizations/services/organizations.membership.service.js | Implements pending visibility, last-owner scope fix, invitation email in addMember, and declineMembership. |
| modules/organizations/routes/organizations.membershipRequest.routes.js | Adds authenticated decline route for pending owner_add invitations. |
| modules/organizations/controllers/organizations.membershipRequest.controller.js | Adds decline controller that returns opaque 404 on any consent mismatch. |
| modules/organizations/tests/organizations.membership.silent.catch.unit.tests.js | Extends silent-catch logging tests to cover addMember email failures. |
| modules/organizations/tests/organizations.membership.lifecycle.unit.tests.js | Unit tests for decline consent gate, last-owner scope, and list filter behavior. |
| modules/organizations/tests/organizations.membership.addMember.email.unit.tests.js | Unit tests for addMember invitation email behavior across configured/unconfigured/no-email cases. |
| modules/organizations/tests/organizations.decline.e2e.tests.js | E2E test covering full decline flow and re-request-to-join behavior. |
| config/templates/org-member-added.html | Adds email template used by the new addMember invitation notification. |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/templates/org-member-added.html`:
- Line 2: The template has an empty <title></title> in org-member-added.html
which fails lint; update the <title> element to a meaningful, non-empty string
(e.g., "You're invited to join {{orgName}}" or another appropriate template
variable) so the HTML title-require rule passes; modify the <title> tag in the
file to include that static text or interpolation and keep any existing head
structure unchanged.
In `@modules/organizations/services/organizations.membership.service.js`:
- Around line 491-500: Current code reads the membership with
MembershipRepository.get and later calls MembershipRepository.remove, which
risks a race where the membership's status changes between read and delete;
replace the two-step read+remove with a single atomic delete constrained by _id
+ userId + status:PENDING + source:OWNER_ADD (use membershipId and
decliningUserId and MEMBERSHIP_STATUSES.PENDING + PENDING_SOURCES.OWNER_ADD) so
the delete only succeeds if the document is still pending-owner-add and returns
the removed document (or null) in one operation (e.g., use a findOneAndDelete /
deleteOne returning document method on MembershipRepository or add an atomic
method to the repository if missing), and then return that result instead of
calling remove after get.
In `@modules/organizations/tests/organizations.decline.e2e.tests.js`:
- Around line 25-36: The JSDoc for the named helper functions is missing
`@returns` tags; update the JSDoc for resetOrgConfig to include an explicit
"`@returns` {void} Resets organizations config to original state." and update the
JSDoc for the async cleanupUser helper to include an explicit "`@returns`
{Promise<void>} Resolves when the user and associated organizations/memberships
are cleaned up." — ensure these tags are added to the existing comment blocks
above the resetOrgConfig and cleanupUser function declarations.
In
`@modules/organizations/tests/organizations.membership.addMember.email.unit.tests.js`:
- Around line 100-117: Add assertions to ensure org DB reads are skipped in the
two tests that verify sendMail is not called: in the test named "mailer NOT
configured: creates the membership and never calls sendMail" and in the test
named "user without an email: creates the membership and never calls sendMail",
add expect(mockOrgGet).not.toHaveBeenCalled() after the existing
expect(mockSendMail).not.toHaveBeenCalled(); this locks in the DB-read guard so
MembershipService.addMember (the function under test) cannot silently call
mockOrgGet when emails are not being sent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4c9a5334-7b95-465e-b1fc-ad0ba5afd353
📒 Files selected for processing (10)
config/templates/org-member-added.htmlmodules/organizations/controllers/organizations.membershipRequest.controller.jsmodules/organizations/routes/organizations.membershipRequest.routes.jsmodules/organizations/services/organizations.membership.service.jsmodules/organizations/tests/organizations.decline.e2e.tests.jsmodules/organizations/tests/organizations.membership.addMember.email.unit.tests.jsmodules/organizations/tests/organizations.membership.lifecycle.unit.tests.jsmodules/organizations/tests/organizations.membership.silent.catch.unit.tests.jsmodules/users/services/users.service.jsmodules/users/tests/users.service.remove.pendingSweep.unit.tests.js
- email template: add non-empty <title>{{appName}} invitation</title>
(fixes title-require lint error flagged by CodeRabbit/HTMLHint)
- addMember: wrap OrganizationRepository.get() in try/catch so any DB
error inside the notification block never propagates to the caller;
the fire-and-forget guarantee now covers the org-fetch step too
(Copilot finding — line 421)
- declineMembership: replace read-then-delete with atomic
MembershipRepository.findOneAndDelete({_id, userId, status:PENDING,
source:OWNER_ADD}) — eliminates the accept/decline race window where a
row activated between get() and remove() could be wrongly deleted
(CodeRabbit major finding — line 500); add findOneAndDelete() to repo
- lifecycle unit tests: update declineMembership suite to mock/assert
the new findOneAndDelete path; add concurrent-accept race scenario
- membershipRequest controller unit tests: add 4 decline() cases
(happy path, null→404, id fallback, throw→422) to close the 7-line
patch coverage gap reported by codecov
- addMember email unit tests: add expect(mockOrgGet).not.toHaveBeenCalled()
to the two skip-email branches (CodeRabbit nit — line 117)
- decline e2e tests: add @returns JSDoc tags to resetOrgConfig and
cleanupUser named helpers (CodeRabbit nit — line 36)
refs #3831
refs #3832
Summary
Node half of the owner_add membership lifecycle (cross-stack pair — the Vue surfaces land in a follow-up PR that closes these issues).
#3831 — owner_add lifecycle:
DELETE /api/membership-requests/:membershipId(auth-only) +MembershipService.declineMembership()— consent gate identical toacceptMembership(pending +owner_add+ caller is the invitee), opaque 404 on any mismatch. The "Please accept or decline it" copy is finally honest.remove()'s last-owner protection now only fires forstatus === ACTIVEowner rows — cancelling a pending owner-invite in a 1-active-owner org no longer spuriously throws.owner_addrows ($orof ACTIVE + PENDING/owner_add); pending join_requests stay on their own approval surface.users.service remove()now sweeps the deleted user's PENDING membership rows (both sources) so they don't orphan.#3832 — invitee email:
addMember()sends a fire-and-forget invitation email (newconfig/templates/org-member-added.html, pending-acceptance wording — never "you were added"),mailer.isConfigured-gated,.catch(logger.warn), never failing the add. (The cap-error wording half of #3832 is frontend-only — it ships with the Vue PR.)Test plan
5 new suites: decline e2e (consent gate), membership lifecycle unit, addMember email unit, silent-catch unit, user-delete pending-sweep unit. Full
organizations.membershipset: 8 suites / 86 tests green. Lint clean.Guardrails
npm run lintcleanrefs #3831
refs #3832
Summary by CodeRabbit
New Features
Bug Fixes